Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove misleading sentence about incompatible QUIC versions and ALPN #2802

Merged
merged 2 commits into from
Sep 10, 2019

Conversation

marten-seemann
Copy link
Contributor

The server doesn't pick a version. The server only picks the application protocol from what the client offered, and it might do that based on the QUIC version that the client picked.

@ianswett ianswett added the -tls label Jun 18, 2019
@@ -1279,8 +1279,7 @@ An application-layer protocol MAY restrict the QUIC versions that it can operate
over. Servers MUST select an application protocol compatible with the QUIC
Copy link
Contributor

@mikkelfj mikkelfj Jun 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plural singular mix: Servers vs the server. Perhaps use "a server"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not part of this PR.

Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This text still serves a purpose because the server does pick ALPN.

@marten-seemann
Copy link
Contributor Author

@martinthomson That depends on how you read RFC 7301, doesn't it?

Section 3.1 says:

Servers that receive a ClientHello containing the "application_layer_protocol_negotiation" extension MAY return a suitable protocol selection response to the client.

I interpret "suitable" as "one of the options that the client proposed", but unfortunately that RFC is not very specific.

@martinthomson
Copy link
Member

That is confusing, but it doesn't mean that the server doesn't pick an ALPN. We do require that it select an ALPN value, and even if it picks one that the client doesn't offer (whether legitimate or not), that doesn't invalidate this text.

Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removing the sentence entirely is the wrong approach.

@@ -1279,8 +1279,7 @@ An application-layer protocol MAY restrict the QUIC versions that it can operate
over. Servers MUST select an application protocol compatible with the QUIC
version that the client has selected. If the server cannot select a compatible
combination of application protocol and QUIC version, it MUST abort the
connection. A client MUST abort a connection if the server picks an incompatible
combination of QUIC version and ALPN identifier.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requirement is still true. If you object to the implication of the server picking a version, maybe this should be "A client MUST abort a connection if the server picks an application protocol incompatible with the protocol version being used."?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that one. Just updated the PR.

@MikeBishop
Copy link
Contributor

No, I was suggesting restoring the sentence that was there before, but with alternate wording to address @marten-seemann's concern. Now the server MUST abort if it can't pick a proper combination, and the client MUST abort if the server fails to abide by that requirement.

Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see now. Yes, this WFM.

@martinthomson martinthomson added the editorial An issue that does not affect the design of the protocol; does not require consensus. label Sep 10, 2019
@martinthomson martinthomson merged commit f62d248 into quicwg:master Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-tls editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants